[Frontend] Remove experiment tracking #2212

tynandebold · 2024-11-29T12:25:09Z

Description

Resolves #2207

Development notes

Removed all frontend references to experiment tracking.

Still to discuss if we want to:

Remove all routing and react-router-dom
Remove the sanitizedPathname() function

QA notes

Test the app locally. Everything should be the same, just without experiment tracking.

Checklist

Read the contributing guidelines
Opened this PR as a 'Draft Pull Request' if it is work-in-progress
Updated the documentation to reflect the code changes
Added new entries to the RELEASE.md file
Added tests to cover my changes

…rimentsIcon Signed-off-by: tynandebold <[email protected]>

Signed-off-by: tynandebold <[email protected]>

…move-experiment-tracking Signed-off-by: tynandebold <[email protected]>

Huongg · 2024-12-02T10:16:30Z

Hey, thanks for putting together this PR @tynandebold ! I'll take some time to review it this morning since it's still a draft. Here are my thoughts on the two points you mentioned:

sanitizedPathname: I think we can remove this since we no longer need to switch between two routes, and it doesn’t seem like it would cause any breaking changes.
react-router-dom: My initial thought is yes, we could remove it since we don't have multiple routes anymore. However, we still use some features from it, like matchPath and useHistoryfor dynamic URLs. Removing it might require refactoring for the URL handling. My gut feeling is that we should treat this separately and not include it in the current scope.

Let me know you thoughts 😄

Huongg · 2024-12-02T10:19:32Z

Also a question for @stephkaiser, since we're removing experiment tracking from the navigation bar, does it still make sense to highlight the flowchart?

Huongg · 2024-12-03T13:21:48Z

hey overall the changes look good to me, i tested it locally and everything seems okay, apart from /experiment-tracking is still loading and not throwing error. I'll do another scan again after I've done with my current PR. Thank you very much @tynandebold

Updates the requirements on [moto](https://github.com/getmoto/moto) to permit the latest version. - [Release notes](https://github.com/getmoto/moto/releases) - [Changelog](https://github.com/getmoto/moto/blob/master/CHANGELOG.md) - [Commits](getmoto/moto@5.0.9...5.0.21) --- updated-dependencies: - dependency-name: moto dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Updates the requirements on [httpx](https://github.com/encode/httpx) to permit the latest version. - [Release notes](https://github.com/encode/httpx/releases) - [Changelog](https://github.com/encode/httpx/blob/master/CHANGELOG.md) - [Commits](encode/httpx@0.27.0...0.28.0) --- updated-dependencies: - dependency-name: httpx dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Updates the requirements on [bandit](https://github.com/PyCQA/bandit) to permit the latest version. - [Release notes](https://github.com/PyCQA/bandit/releases) - [Commits](PyCQA/bandit@1.7.0...1.8.0) --- updated-dependencies: - dependency-name: bandit dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Updates the requirements on [boto3](https://github.com/boto/boto3) to permit the latest version. - [Release notes](https://github.com/boto/boto3/releases) - [Commits](boto/boto3@1.34.0...1.35.71) --- updated-dependencies: - dependency-name: boto3 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jitendra Gundaniya <[email protected]>

jitu5 · 2024-12-04T07:12:47Z

@tynandebold I have reviewed it and tested locally as well and all looks good. Just few note,

If we hit /experiment-tracking url directly as Huong mentioned its not throwing error.
sanitizedPathname is still there in utils, if we are planning to remove it.
Recent fix related to 404 Github pages issue, we need to revert that as well.

…move-experiment-tracking Signed-off-by: tynandebold <[email protected]>

Signed-off-by: tynandebold <[email protected]>

tynandebold · 2024-12-04T13:25:37Z

.gitignore

+# Python
+.venv/
+mise.toml


I've added this because I'm using mise to manage my Python environment, not conda.

I was a mise user as well for a while! Until I discovered https://docs.astral.sh/uv/ 😄

Huongg

Hey thank you @tynandebold, amazing work and super quick turnaround! Everything looks great to me except for one small NIT point. It's all running okay on my local setup as well. As we discussed, /experiment-tracking doesn’t throw any errors on localhost. We’ll revisit this once I complete the backend removal, and we can re-test it on the feature_branch. I'll leave the note to remind us to test it in the feature_branch

src/store/initial-state.js

jitu5

Thanks @tynandebold Amazing...!

astrojuanlu · 2024-12-05T07:25:22Z

Thank you @tynandebold !

We received feedback from some team members that the decision to proceed with the removal was a bit opaque. So in light of the importance of this step, I think the right call is to request approval from the TSC. It will delay the PR a bit but does everyone agree?

jitu5 · 2024-12-05T07:28:05Z

Thank you @tynandebold !

We received feedback from some team members that the decision to proceed with the removal was a bit opaque. So in light of the importance of this step, I think the right call is to request approval from the TSC. It will delay the PR a bit but does everyone agree?

@astrojuanlu Make sense to include TSC.

Huongg · 2024-12-05T08:33:52Z

I think the right call is to request approval from the TSC. It will delay the PR a bit but does everyone agree?

hey @astrojuanlu That's a good idea to get approval from the TSC, the question is there will be at least 3-5 more PRs as a part of this work. Do we add them in every PR? Or should we add them to the feature_branch which will have the final changes both back-end, front-end, docs etc.

tynandebold · 2024-12-05T10:24:25Z

I think the right call is to request approval from the TSC. It will delay the PR a bit but does everyone agree?

hey @astrojuanlu That's a good idea to get approval from the TSC, the question is there will be at least 3-5 more PRs as a part of this work. Do we add them in every PR? Or should we add them to the feature_branch which will have the final changes both back-end, front-end, docs etc.

A TSC vote makes sense and so does your point, Huong.

I think the TSC approval should be on the feature_branch/remove-experiment-tracking branch, which will be the final, all-encompassing PR.

Signed-off-by: tynandebold <[email protected]>

SajidAlamQB

Thanks for this, I've tested it manually and all seems good!

astrojuanlu · 2024-12-05T14:10:12Z

I think the TSC approval should be on the feature_branch/remove-experiment-tracking branch, which will be the final, all-encompassing PR.

Sounds good, thanks 👍🏼 please keep it in mind before submitting the final PR to main

tynandebold added 4 commits November 22, 2024 17:15

Remove exp tracking from feature hits and global toolbar; remove Expe…

86cecbb

…rimentsIcon Signed-off-by: tynandebold <[email protected]>

Remove /experiment-tracking directory; fix tests

72ebf34

Signed-off-by: tynandebold <[email protected]>

Remove exp. tracking utils, reducers, styles

6974f07

Signed-off-by: tynandebold <[email protected]>

Merge branch 'main' of github.com:kedro-org/kedro-viz into feature/re…

75e15ef

…move-experiment-tracking Signed-off-by: tynandebold <[email protected]>

tynandebold requested review from rashidakanchwala, astrojuanlu, ravi-kumar-pilla and jitu5 as code owners November 29, 2024 12:25

tynandebold requested review from Huongg and removed request for astrojuanlu, rashidakanchwala, jitu5 and ravi-kumar-pilla November 29, 2024 12:25

tynandebold marked this pull request as draft November 29, 2024 13:34

dependabot bot and others added 4 commits December 3, 2024 13:34

tynandebold added 4 commits December 4, 2024 12:08

Merge branch 'main' of github.com:kedro-org/kedro-viz into feature/re…

ac83853

…move-experiment-tracking Signed-off-by: tynandebold <[email protected]>

Revert change from PR #2179

e12226c

Signed-off-by: tynandebold <[email protected]>

Remove the sanitizedPathname() function

193f1dc

Signed-off-by: tynandebold <[email protected]>

Remove Cypress tests for exp tracking

ee33875

Signed-off-by: tynandebold <[email protected]>

tynandebold marked this pull request as ready for review December 4, 2024 12:23

tynandebold requested a review from jitu5 December 4, 2024 12:24

tynandebold commented Dec 4, 2024

View reviewed changes

Huongg approved these changes Dec 4, 2024

View reviewed changes

src/store/initial-state.js Outdated Show resolved Hide resolved

jitu5 approved these changes Dec 5, 2024

View reviewed changes

Merge localStorageState state instead of the older allLocalStorageState

dcff28d

Signed-off-by: tynandebold <[email protected]>

SajidAlamQB approved these changes Dec 5, 2024

View reviewed changes

Huongg merged commit 2cc3aad into feature_branch/remove-experiment-tracking Dec 6, 2024
29 checks passed

Huongg deleted the feature/remove-experiment-tracking branch December 6, 2024 11:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Frontend] Remove experiment tracking #2212

[Frontend] Remove experiment tracking #2212

tynandebold commented Nov 29, 2024 •

edited

Loading

Huongg commented Dec 2, 2024

Huongg commented Dec 2, 2024

Huongg commented Dec 3, 2024

jitu5 commented Dec 4, 2024

tynandebold Dec 4, 2024

astrojuanlu Dec 5, 2024

Huongg left a comment

jitu5 left a comment

astrojuanlu commented Dec 5, 2024

jitu5 commented Dec 5, 2024

Huongg commented Dec 5, 2024 •

edited

Loading

tynandebold commented Dec 5, 2024

SajidAlamQB left a comment

astrojuanlu commented Dec 5, 2024

[Frontend] Remove experiment tracking #2212

[Frontend] Remove experiment tracking #2212

Conversation

tynandebold commented Nov 29, 2024 • edited Loading

Description

Development notes

QA notes

Checklist

Huongg commented Dec 2, 2024

Huongg commented Dec 2, 2024

Huongg commented Dec 3, 2024

jitu5 commented Dec 4, 2024

tynandebold Dec 4, 2024

Choose a reason for hiding this comment

astrojuanlu Dec 5, 2024

Choose a reason for hiding this comment

Huongg left a comment

Choose a reason for hiding this comment

jitu5 left a comment

Choose a reason for hiding this comment

astrojuanlu commented Dec 5, 2024

jitu5 commented Dec 5, 2024

Huongg commented Dec 5, 2024 • edited Loading

tynandebold commented Dec 5, 2024

SajidAlamQB left a comment

Choose a reason for hiding this comment

astrojuanlu commented Dec 5, 2024

tynandebold commented Nov 29, 2024 •

edited

Loading

Huongg commented Dec 5, 2024 •

edited

Loading